checkout: Don't set dir mtime to 0 when doing a force copy checkout

When we [switched to using checkout + force_copy](https://github.com/ostreedev/ostree/commit/e8efd1c8dcaad8fbd3b05c400972d237406263e7),
a side effect that went unnoticed at the time is that we started
setting directory mtimes to zero.

See the below bug where we long ago set the file times to zero, which got fixed,
so let's not regress things by setting the directory times to zero either. (Even
though AFAICS GNU tar doesn't complain about those)

This semantic is somewhat "overloaded" onto `force_copy`, but it avoids adding
yet another boolean; we don't have that many reserved boolean slots left. I
can't really think of many good use cases for `force_copy` *other* than the
`/etc` merge anyways.

https://bugzilla.redhat.com/show_bug.cgi?id=1229160

Closes: https://github.com/ostreedev/ostree/issues/995
Closes: #997
Approved by: jlebon

diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c
index 9ab9a62392ed14cb85e0ec0afc32f358f7e3579c..7942d60745cf302319f88d178eff83de46a29c59 100644 (file)
--- a/src/libostree/ostree-repo-checkout.c
+++ b/src/libostree/ostree-repo-checkout.c
@@ -770,9 +770,11 @@ checkout_tree_at_recurse (OstreeRepo                        *self,
     }
 
   /* Set directory mtime to OSTREE_TIMESTAMP, so that it is constant for all checkouts.
-   * Must be done after setting permissions and creating all children.
+   * Must be done after setting permissions and creating all children.  Note we skip doing
+   * this for directories that already exist (under the theory we possibly don't own them),
+   * and we also skip it if doing copying checkouts, which is mostly for /etc.
    */
-  if (!did_exist)
+  if (!did_exist && !options->force_copy)
     {
       const struct timespec times[2] = { { OSTREE_TIMESTAMP, UTIME_OMIT }, { OSTREE_TIMESTAMP, 0} };
       if (TEMP_FAILURE_RETRY (futimens (destination_dfd, times)) < 0)

diff --git a/tests/installed/itest-deploy-selinux.sh b/tests/installed/itest-deploy-selinux.sh
index 7c26aad02ac26a58a88c309d0e458292b0cb9ac9..f490b69d421a4a849830d1bb31ad7b0e13b2a8f2 100755 (executable)
--- a/tests/installed/itest-deploy-selinux.sh
+++ b/tests/installed/itest-deploy-selinux.sh
@@ -11,6 +11,12 @@ dn=$(dirname $0)
 ostree admin deploy --karg-proc-cmdline ${host_refspec}
 new_deployment_path=/ostree/deploy/${host_osname}/deploy/${host_commit}.1
 
+# Test /etc directory mtime
+if ! test ${new_deployment_path}/etc/NetworkManager -nt /etc/NetworkManager; then
+    ls -al ${new_deployment_path}/etc/NetworkManager /etc/NetworkManager
+    fatal "/etc directory mtime not newer"
+fi
+
 # A set of files that have a variety of security contexts
 for file in fstab passwd exports hostname sysctl.conf yum.repos.d \
             NetworkManager/dispatcher.d/hook-network-manager; do